Skip to content

Conversation

@Erich-McMillan
Copy link

@Erich-McMillan Erich-McMillan commented Dec 14, 2022

Summary

Addresses concerns from PR #47 , by

  1. Allowing backward compatibility with previous interface which only supported a single class while still supporting a list of classes.
  2. Updating documentation to include section on serving multiple libraries with a single server.

Also updates property from _library to _libraries to improve clarity for reader.

  • new functionality
  • breaking changes
  • documentation

@Erich-McMillan
Copy link
Author

@srinivaschavan this PR supersedes #47 per your request to create a new PR, review feedback is appreciated :)


if __name__ == '__main__':
RobotRemoteServer(ExampleLibrary(), *sys.argv[1:])
RobotRemoteServer([ExampleLibrary()], *sys.argv[1:])
Copy link
Author

@Erich-McMillan Erich-McMillan Dec 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Felt it is best to encourage/guide any new users of the remote server to use list in the example to avoid confusion going forward. Let me know if you think we should remove the list from this example.

Copy link

@srinivaschavan srinivaschavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Erich-McMillan
Copy link
Author

@srinivaschavan, thanks :) I don't have merge permissions, any idea when this can be merged in?

@srinivaschavan
Copy link

@srinivaschavan, thanks :) I don't have merge permissions, any idea when this can be merged in?

I do not have the merge permissions either. Maybe we need approval from @pekkaklarck

@pekkaklarck
Copy link
Member

Thanks for the PR! On a super quick look it looks good. My plan is to release v1.1.1 now and then early next year look at v1.2 where this could be added.

As an APi giving libs to expose as a list sounds good. From Robot's point of view that then creates a single big library with all keywords combined.

Another thing I have been thinking is registering different libraries under different paths so that you could use e.g. http://localhost:8270/lib1 and http://localhost:8270/lib2 then taking libraries into use. In that case you'd pass libraries as a dictionary like {'lib1': Library1(), 'lib2': Library2()}. I don't know how easy registering different paths like that is, though.

@Erich-McMillan
Copy link
Author

Thanks for the PR! On a super quick look it looks good. My plan is to release v1.1.1 now and then early next year look at v1.2 where this could be added.

As an APi giving libs to expose as a list sounds good. From Robot's point of view that then creates a single big library with all keywords combined.

Another thing I have been thinking is registering different libraries under different paths so that you could use e.g. http://localhost:8270/lib1 and http://localhost:8270/lib2 then taking libraries into use. In that case you'd pass libraries as a dictionary like {'lib1': Library1(), 'lib2': Library2()}. I don't know how easy registering different paths like that is, though.

I like the idea of serving under different url paths, though I'm not sure how that would change to the high level Library Remote url import or keyword usage; would that mean using a separate remote instance per library being severed like the following example?

*** Settings ***
Library  Remote  http://localhost:8270/lib1
Library  Remote  http://localhost:8270/lib2

@pekkaklarck
Copy link
Member

Yes, you needed to take those libraries into use separately. That way you could give them separate aliases when importing using AS (or deprecated WITH NAME). I would assume that's sometimes preferred over effectively having all keywords in same library. An alternative is running multiple remote server instances, but that would require starting them separately as well.

@Erich-McMillan
Copy link
Author

In this case it may be ideal to implement a new function get_libraries/get_library (which would return a library object with get_keywords as part of the xml server? Thus the import would be Library Remote http://localhost:8270 library=lib1.

In my case I am not directly using the Remote library in .robot files but instead using it internally to another library written in python; so being able to dynamically detect+import all the libraries without user specification is desirable. Perhaps to maintain backward compatibility we could have Library Remote http://localhost:8270 import all libraries exposed by the xml server?

As for how involved implementing this will be, I'm not certain.

@HackXIt
Copy link

HackXIt commented Feb 17, 2025

What is missing to merge this PR?

It appears the repo is quite dead weight by now, but I really need this functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants